sql: set default for large_full_scan_rows to 0 #131040
Conversation
michae2
requested review from
a team and
mw5h
and removed request for
a team
|
This change is |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @michae2 and @mw5h)
-- commits line 14 at r1:
We should probably update the docs here to make it ultra clear what the defaults of large_full_scan_rows are within the description of disallow_full_table_scans and what the implications are when enabling disallow_full_table_scans: that ALL queries with a full table scan will result in an error. cc @taroface I believe those descriptions are owned by the docs team, and not in code, but correct me if I'm wrong.
pkg/sql/exec_util.go line 714 at r1 (raw file):
"default value for large_full_scan_rows session setting which determines "+ "the maximum table size allowed for a full scan when disallow_full_table_scans "+ "is set to true",
nit: add something like this helpful bit from the session setting docs: "To reject all full table or index scans, set to 0."
That's correct -- since this PR has a release note, it will generate a corresponding DOC (product change) issue when merged. The doc update will be tracked in that issue. |
michae2
force-pushed
the
largefullscans
branch
from
967996c
to
9290096
Compare
michae2
force-pushed
the
largefullscans
branch
from
9290096
to
122b92d
Compare
In cockroachdb#69371 we set the default for `large_full_scan_rows` to 1000. This default can be surprising for users of `disallow_full_table_scans`. Now that cockroachdb#122708 is fixed, let's set the default to 0 so that the exemption for small tables is opt-in rather than opt-out. Fixes: cockroachdb#130726 Release note (sql change): Set the default for session variable `large_full_scan_rows` to 0. This means that by default, `disallow_full_table_scans` will disallow _all_ full table scans, even full scans on very small tables. If `large_full_scan_rows` is set > 0, `disallow_full_table_scans` will allow full scans estimated to read fewer than `large_full_scan_rows`.
michae2
force-pushed
the
largefullscans
branch
from
122b92d
to
10b9ca1
Compare
There was a problem hiding this comment.
TFTR!
bors r=mgartner
Reviewable status:
Previously, mgartner (Marcus Gartner) wrote…
We should probably update the docs here to make it ultra clear what the defaults of
large_full_scan_rowsare within the description ofdisallow_full_table_scansand what the implications are when enablingdisallow_full_table_scans: that ALL queries with a full table scan will result in an error. cc @taroface I believe those descriptions are owned by the docs team, and not in code, but correct me if I'm wrong.
This one is generated, so I went ahead and updated it.
pkg/sql/exec_util.go line 714 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
nit: add something like this helpful bit from the session setting docs: "To reject all full table or index scans, set to
0."
good call, done
|
|
In #69371 we set the default for
large_full_scan_rowsto 1000. This default can be surprising for users ofdisallow_full_table_scans. Now that #122708 is fixed, let's set the default to 0 so that the exemption for small tables is opt-in rather than opt-out.Fixes: #130726
Release note (sql change): Set the default for session variable
large_full_scan_rowsto 0. This means that by default,disallow_full_table_scanswill disallow all full table scans, even full scans on very small tables. Iflarge_full_scan_rowsis set > 0,disallow_full_table_scanswill allow full scans estimated to read fewer thanlarge_full_scan_rows.